[Discover] Support custom logic to insert time filter based on dataset type #8932
Conversation
Signed-off-by: Joshua Li <[email protected]>
Signed-off-by: Joshua Li <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8932 +/- ##
=======================================
Coverage 60.87% 60.88%
=======================================
Files 3802 3802
Lines 91072 91092 +20
Branches 14375 14380 +5
=======================================
+ Hits 55444 55462 +18
- Misses 32086 32088 +2
Partials 3542 3542
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Joshua Li <[email protected]>
joshuali925
force-pushed
the
fix-time-field
branch
from
38165a2 to
a0ea032
Compare
joshuali925
requested review from
ananzh,
kavilla,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs,
ZilongX,
Flyingliuhub,
curq,
bandinib-amzn,
SuZhou-Joe,
ruanyl,
BionIT,
xinruiba,
zhyuanqi,
mengweieric,
LDrago27 and
virajsanghvi
as code owners
joshuali925
changed the title
| @@ -43,6 +43,13 @@ export interface DatasetTypeConfig { | |||
| id: string; | |||
| /** Human-readable title for the dataset type */ | |||
| title: string; | |||
| languageOverrides?: { | |||
There was a problem hiding this comment.
Is there an example of how or when languageOverrides will be used?
There was a problem hiding this comment.
currently no. this is for example if PPL needs different logic to insert the time filter for a specific dataset. Once the dataset overrides this option, PPL will not insert time filter in pplSearchInterceptor, but will pass the timeRange to params.body, and the dataset search strategy would be able to access the user query and timeRange to insert the time filter
for example
languageOverrides: {
PPL: {
hideDatePicker: false,
},
},and in search strategy, the request will contain
const query: Query = request.body.query;
const timeRange: TimeRange = request.body.timeRange;| const dataset = query.dataset; | ||
| if (!dataset || !dataset.timeFieldName) return query; | ||
| const datasetService = queryString.getDatasetService(); | ||
| if (datasetService.getType(dataset.type)?.languageOverrides?.PPL?.hideDatePicker === false) |
There was a problem hiding this comment.
It seems when hideDatePicker === false, timeRange is added to params.body, otherwise, the time filter is encapsulated in the PPL query on line 102. The time filter will anyway be added, is this by intention?
There was a problem hiding this comment.
yes it's by intention, if hideDatePicker === false, then it's not automatically inserted. This allows custom logic to insert the time filter
There was a problem hiding this comment.
I see, so when timeRange is added to params.body, then it's up to server side search strategy to decide how to use it, did I get it right?
| @@ -62,6 +63,16 @@ export class SQLSearchInterceptor extends SearchInterceptor { | |||
| .getDatasetService() | |||
| .getType(datasetType); | |||
| strategy = datasetTypeConfig?.getSearchOptions?.().strategy ?? strategy; | |||
|
|
|||
| if (datasetTypeConfig?.languageOverrides?.SQL?.hideDatePicker === false) { | |||
There was a problem hiding this comment.
Is this just for consistency? Because I don't see it add time range in the query automatically in sql search interceptor.
There was a problem hiding this comment.
the idea is that if there is a language override that sets hideDatePicker: false, then the search interceptor will not automatically insert the timeRange into query, but will pass timeRange to search strategy. Passing timeRange happens regardless of whether the search interceptor has logic to insert timeRange
Currently we don't have SQL query parser + builder so it's hard to insert time range to SQL, but if the search API for a specific dataset takes a SQL query and a separate time range parameter, then this will be useful
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-8932-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 7df73ddeea9eb8f0c462cc8a099dc32f49d14692
# Push it to GitHub
git push --set-upstream origin backport/backport-8932-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.xThen, create a pull request where the |
|
backport 2.x depends on #8901 |
…t type (opensearch-project#8932) * Pass time filter if language overrides hideDatePicker --------- Signed-off-by: Joshua Li <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Signed-off-by: Federico Silva <[email protected]>
Description
Continue on #8917
This PR
hideDatePickerper language for custom logic to insert time filter into queryIssues Resolved
closes #8917
Screenshot
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integration